Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fixed version detection on linux #1253

Merged
merged 5 commits into from
Apr 10, 2021
Merged

fixed version detection on linux #1253

merged 5 commits into from
Apr 10, 2021

Conversation

xnetcat
Copy link
Member

@xnetcat xnetcat commented Apr 7, 2021 •
edited
Loading

For some reason subprocess.Popopen returns tuple (proc_err, proc_out) instead of (proc_out, proc_err) on linux.

I have no clue why it's happening

Other way of fixing this suggested by @ndgnuh

process = subprocess.Popen(["ffmpeg", "-version"], shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
proc_out = process.stdout.readlines()[0]

process.communicate() return
image

@xnetcat xnetcat requested review from Silverarmor and aklajnert and removed request for Silverarmor April 7, 2021 15:07
@Silverarmor
Copy link
Member

Tests are getting Your FFmpeg version couldn't be detected

aklajnert
aklajnert requested changes Apr 8, 2021
View reviewed changes
spotdl/download/ffmpeg.py Outdated Show resolved Hide resolved
@xnetcat
try to find ffmpeg version in both proc_out and proc_err
d8e3a9d 
ffmpeg on Windows writes its output to stdout and on Linux to stderr
aklajnert
aklajnert reviewed Apr 8, 2021
View reviewed changes
spotdl/download/ffmpeg.py Outdated Show resolved Hide resolved
@xnetcat xnetcat changed the base branch from master to dev April 8, 2021 06:15
@xnetcat
Copy link
Member Author

xnetcat commented Apr 8, 2021

image

@aklajnert text=True is not supported in python3.6. Should I create process without text parameter before python3.7 and set encoding instead?

https://docs.python.org/3.6/library/subprocess.html

If encoding or errors are specified, or universal_newlines is true, file objects for stdin, stdout and stderr are opened in text mode using the specified encoding and errors or the io.TextIOWrapper default. Otherwise, file objects are opened in binary mode.

image

@aklajnert
Copy link
Contributor

Setting encoding will have probably the same result in 3.6 and 3.7 so maybe just replace text=True with encoding="utf-8"?
Or try the join() by bytes (b"") trick.

@xnetcat
Copy link
Member Author

xnetcat commented Apr 8, 2021

Setting encoding will have probably the same result in 3.6 and 3.7 so maybe just replace text=True with encoding="utf-8"?
Or try the join() by bytes (b"") trick.

Setting encoding will open stdin, stdout and stderr in text mode on all versions of python

@xnetcat xnetcat mentioned this pull request Apr 8, 2021
Closed
@xnetcat xnetcat requested a review from aklajnert April 10, 2021 17:42
@Silverarmor
Copy link
Member

Please pull changes from upstream/dev

@xnetcat
Copy link
Member Author

xnetcat commented Apr 10, 2021

Please pull changes from upstream/dev

Done

@Silverarmor Silverarmor merged commit 9fac267 into spotDL:dev Apr 10, 2021
@xnetcat xnetcat deleted the linux-subprocess-fix branch April 10, 2021 22:47
@Silverarmor Silverarmor added the Bug Fix PRs that fix bugs label Apr 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aklajnert aklajnert aklajnert approved these changes

@Silverarmor Silverarmor Awaiting requested review from Silverarmor

Assignees
No one assigned
Labels
Bug Fix PRs that fix bugs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xnetcat @Silverarmor @aklajnert